Skip to content

Add problem areas: Tekton pipeline review, migration path, multi-tenancy - #2

Closed
vdemeester wants to merge 1 commit into
fullsend-ai:mainfrom
vdemeester:vdemeester/problem-areas
Closed

Add problem areas: Tekton pipeline review, migration path, multi-tenancy#2
vdemeester wants to merge 1 commit into
fullsend-ai:mainfrom
vdemeester:vdemeester/problem-areas

Conversation

@vdemeester

Copy link
Copy Markdown

What

Three new problem documents and practitioner perspective additions to the code review doc.

New problem areas

Tekton Pipeline Review — Reviewing Tekton task/pipeline definitions is a distinct discipline from reviewing application code. Build-definitions is the most critical repo in the org and it's almost entirely Tekton YAML with embedded shell. This doc covers:

  • Embedded shell scripts in YAML (bash semantics + YAML embedding + Tekton parameter substitution)
  • Stringly-typed interfaces (result propagation, parameter threading, workspace bindings)
  • The trusted task model and its security implications for review
  • Backwards compatibility and versioning concerns
  • What a pipeline review agent needs to understand

Migration Path — How to get from today's workflow to agent-driven development incrementally. Proposes 5 phases:

  • Phase 0: Observation (readiness assessment, baseline data)
  • Phase 1: Shadow review (agents comment, humans still decide)
  • Phase 2: Assisted review (agent review required, human merge)
  • Phase 3: Conditional autonomy (Tier 0 auto-merge on graduated repos)
  • Phase 4: Full autonomy for graduated repos

Includes repo selection criteria, bootstrap requirements, and anti-patterns.

Multi-tenancy — Konflux is multi-tenant. Agents need to understand tenant isolation boundaries to avoid introducing multi-tenancy bugs. Covers:

  • The dual nature of multi-tenancy (workspace isolation + build-time isolation)
  • Common multi-tenancy bug patterns (missing namespace scoping, cross-namespace references, shared state without isolation)
  • The testing gap (test coverage numbers overstate readiness for multi-tenancy correctness)
  • How multi-tenancy fits in the sub-agent model

Additions to existing docs

Code Review — Added two new sections:

  • The heterogeneous codebase problem — the sub-agent model assumes Go code, but the org is heterogeneous (Go, Tekton YAML, shell, React, Python). Different content types need different review approaches.
  • What human reviewers actually catch (and miss) — grounding the sub-agent model in real review experience. What agents will struggle with ("this works but it's the wrong approach"), what agents could be better at (consistent convention enforcement, cross-file impact analysis).

Updated README.md and CLAUDE.md to link the new docs.

Add three new problem documents and expand the code review doc with
practitioner perspectives:

- tekton-pipeline-review.md: Reviewing Tekton task/pipeline definitions
  as a distinct domain — embedded shell, stringly-typed interfaces,
  result propagation, trusted task model, cross-cutting concerns
- migration-path.md: Phased approach from current human-driven workflow
  to agent-driven development (observation → shadow → assisted →
  conditional autonomy → full autonomy)
- multi-tenancy.md: How agents understand and preserve tenant isolation
  boundaries — namespace scoping, build-time isolation, the testing gap
- code-review.md: Add sections on heterogeneous codebase problem, what
  human reviewers actually catch (and miss), and implications for
  sub-agent context loading
@vdemeester vdemeester closed this Mar 9, 2026
ascerra added a commit to ascerra/fullsend that referenced this pull request May 14, 2026
When a human submits a PR and the review agent requests changes, the fix
agent no longer auto-triggers. This is now the default behavior for
human-authored PRs. Bot-authored PRs (from the code agent) continue to
auto-trigger fixes as before.

Humans can opt in to the review-fix loop by adding the `fullsend-fix`
label to their PR. The existing `/fix` command for manual invocation is
unaffected.

Changes across all dispatch paths (org-level shim, reusable dispatcher,
per-repo scaffold):
- Gate fix dispatch on PR author ending in [bot] OR presence of
  `fullsend-fix` label
- Add defense-in-depth check in reusable-fix.yml that verifies PR
  author and label before running the fix agent

E2E tested in ascerra-fullsend-lab org (human-pr-fix-test repo):

1. Human PR, no label (PR fullsend-ai#1) — fix agent SKIPPED ✅
   - Review: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25836235094
   - Shim skipped dispatch-fix-bot: https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25836370170

2. Human PR, fullsend-fix label (PR fullsend-ai#2) — fix agent auto-triggered ✅
   - Review: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25837485822
   - Shim dispatched fix: https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25837620440
   - Fix agent succeeded: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25837623950

3. Bot PR, no label (PR fullsend-ai#4, author: ascerra-fullsend-lab-coder[bot]) — fix agent auto-triggered ✅
   - Code agent created PR: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838129538
   - Review requested changes: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838396492
   - Shim dispatched fix (no label needed): https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25838580565
   - Fix agent succeeded: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838584099

Co-authored-by: Cursor <cursoragent@cursor.com>
ascerra pushed a commit to ascerra/fullsend that referenced this pull request May 16, 2026
Medium:
- Updated PR description to reflect that pre-existing plan links were
  removed from README (no longer claims to link agent-execution-environment,
  vertex-inference-provisioning, and drift-scanner)

Low fullsend-ai#2:
- Removed "Minimal wrapper pattern" section from plan document as it
  argues against itself - describes a pattern that requires capabilities
  that don't exist yet, then recommends removing it

Low fullsend-ai#3:
- Updated IsURL function comment to note that url.Parse may not set
  u.User for all userinfo edge cases, advising implementers to consider
  additional validation if strict userinfo rejection is required

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ascerra added a commit to ascerra/fullsend that referenced this pull request May 16, 2026
When a human submits a PR and the review agent requests changes, the fix
agent no longer auto-triggers. This is now the default behavior for
human-authored PRs. Bot-authored PRs (from the code agent) continue to
auto-trigger fixes as before.

Humans can opt in to the review-fix loop by adding the `fullsend-fix`
label to their PR. The existing `/fs-fix` command for manual invocation
is unaffected — it intentionally bypasses this gate so authorized users
can always trigger fix regardless of labels.

Changes across dispatch paths (reusable dispatcher, per-repo scaffold):
- Gate fix dispatch on PR author ending in [bot] OR presence of
  `fullsend-fix` label
- Add defense-in-depth check in reusable-fix.yml that verifies PR
  author and label before running the fix agent
- Add inline comment clarifying /fs-fix bypass (per review feedback)

Note: fullsend.yaml shim no longer needs this gate — upstream refactored
it to delegate all routing to the reusable dispatch workflow.

E2E tested in ascerra-fullsend-lab org (human-pr-fix-test repo):

1. Human PR, no label (PR fullsend-ai#1) — fix agent SKIPPED
   - Review: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25836235094
   - Shim skipped dispatch-fix-bot: https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25836370170

2. Human PR, fullsend-fix label (PR fullsend-ai#2) — fix agent auto-triggered
   - Review: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25837485822
   - Shim dispatched fix: https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25837620440
   - Fix agent succeeded: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25837623950

3. Bot PR, no label (PR fullsend-ai#4, author: ascerra-fullsend-lab-coder[bot]) — fix agent auto-triggered
   - Code agent created PR: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838129538
   - Review requested changes: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838396492
   - Shim dispatched fix (no label needed): https://github.com/ascerra-fullsend-lab/human-pr-fix-test/actions/runs/25838580565
   - Fix agent succeeded: https://github.com/ascerra-fullsend-lab/.fullsend/actions/runs/25838584099

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant